backend/app/services/intelligence/github/api_client.py (1)

20-24: 💤 Low value

_REPO_PATTERN allows .. as a valid repository name.

The pattern [A-Za-z0-9._-] permits strings like .. or ... which, while likely rejected by GitHub's API, could cause unexpected URL path normalization (e.g., /repos/owner/../ normalizing to /repos/). Consider explicitly excluding consecutive dots or pure-dot strings.

♻️ Optional: Reject consecutive dots in repo names

-_REPO_PATTERN = re.compile(r"^[A-Za-z0-9._-]{1,100}$")
+# Reject names that are only dots or contain ".."
+_REPO_PATTERN = re.compile(r"^(?!\.+$)(?!.*\.\.)[A-Za-z0-9._-]{1,100}$")

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/services/intelligence/github/api_client.py` around lines 20 - 24,
The _REPO_PATTERN currently allows strings like ".."; update its regex to
explicitly reject consecutive dots and names made only of dots by adding
negative lookaheads (e.g., a (?!.*\.\.) to forbid consecutive dots and a
(?!^[.]+$) to forbid pure-dot names) while preserving the existing allowed
characters and length constraints; modify the constant _REPO_PATTERN in
api_client.py (and keep _OWNER_PATTERN unchanged) so repo validation fails for
inputs containing ".." or only dots before any URL interpolation.

backend/tests/security/test_ssrf_github.py (1)

34-41: 💤 Low value

Minor resource leak: event loop created but never closed.

Line 41 creates a new event loop that remains open until process exit. While not problematic for tests, a cleaner pattern would avoid creating the replacement loop or use pytest-asyncio directly.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/tests/security/test_ssrf_github.py` around lines 34 - 41, The helper
_run creates a new event loop then replaces it with another new loop (leaking
one); to fix, avoid creating the replacement loop—either capture the original
loop before creating the temporary one and restore it after closing (e.g., save
original = asyncio.get_event_loop(); set the new loop to run coro; close it;
asyncio.set_event_loop(original)), or simply remove the final
asyncio.set_event_loop(asyncio.new_event_loop()) and call
asyncio.set_event_loop(None) after closing; update the _run function
accordingly.

backend/app/services/tasks/cloud_tasks.py (1)

28-28: ⚡ Quick win

X-Internal-Secret is dead config unless the receiver enforces it.

CloudTasksDispatcher now sends this secret on every callback, but _verify_request() in backend/app/routers/internal.py only checks X-CloudTasks-QueueName plus OIDC. Right now this header adds secret-handling overhead without contributing to authorization, so either validate it server-side or drop it from the request contract.

Also applies to: 35-38

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/services/tasks/cloud_tasks.py` at line 28, CloudTasksDispatcher
is sending X-Internal-Secret but the server-side _verify_request() in
backend/app/routers/internal.py doesn't validate it; either remove the header
from CloudTasksDispatcher or add validation: read the same INTERNAL_SECRET env
var (or internal_secret) in _verify_request() and compare it to the
X-Internal-Secret header value (fail the request if missing/mismatched); update
the verification path used by _verify_request() to include this header check
alongside the existing X-CloudTasks-QueueName and OIDC checks so the header
actually contributes to authorization.

backend/tests/security/test_admin_authorization.py (1)

91-107: ⚡ Quick win

Assert the expected audience in the patched OIDC verifier.

These doubles ignore the audience argument, so the tests still pass if _verify_cloud_tasks_oidc() stops forwarding CLOUD_TASKS_SERVICE_URL into verify_oauth2_token(). That drops coverage for one of the main security properties added in this PR.

Suggested fix

         monkeypatch.setattr(
             "app.routers.internal.id_token.verify_oauth2_token",
-            lambda token, request, audience: {
-                "iss": "https://accounts.google.com",
-                "email": "attacker@example.iam.gserviceaccount.com",
-                "email_verified": True,
-            },
+            lambda token, request, audience: (
+                {
+                    "iss": "https://accounts.google.com",
+                    "email": "attacker@example.iam.gserviceaccount.com",
+                    "email_verified": True,
+                }
+                if audience == "https://backend.example.com"
+                else (_ for _ in ()).throw(AssertionError(f"unexpected audience: {audience}"))
+            ),
         )
@@
         monkeypatch.setattr(
             "app.routers.internal.id_token.verify_oauth2_token",
-            lambda token, request, audience: {
-                "iss": "https://accounts.google.com",
-                "email": "tasks@example.iam.gserviceaccount.com",
-                "email_verified": True,
-            },
+            lambda token, request, audience: (
+                {
+                    "iss": "https://accounts.google.com",
+                    "email": "tasks@example.iam.gserviceaccount.com",
+                    "email_verified": True,
+                }
+                if audience == "https://backend.example.com"
+                else (_ for _ in ()).throw(AssertionError(f"unexpected audience: {audience}"))
+            ),
         )

Also applies to: 119-135

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/tests/security/test_admin_authorization.py` around lines 91 - 107,
The patched OIDC verifier used in the tests (monkeypatch.setattr on
app.routers.internal.id_token.verify_oauth2_token) ignores the audience argument
so the test will pass even if _verify_cloud_tasks_oidc() stops passing
CLOUD_TASKS_SERVICE_URL; update the test doubles to assert or validate that the
incoming audience equals CLOUD_TASKS_SERVICE_URL (or raise an error if it does
not) so the test fails when the verifier is called with the wrong audience,
ensuring the CLOUD_TASKS_SERVICE_URL audience check in
_verify_cloud_tasks_oidc() is covered.

backend/tests/security/test_rate_limit.py (1)

23-35: ⚡ Quick win

Always reset the shared limiter in finally.

These tests mutate global rate-limit state and only clean it up on the success path. If one of the requests or assertions fails early, later tests can inherit a poisoned bucket and start failing nondeterministically.

Suggested fix

-    limiter.reset()
-    statuses: list[int] = []
-    for _ in range(8):
-        resp = client.post(
-            "/api/github-link/run",
-            json={"include_forks": False},
-            headers=headers,
-        )
-        statuses.append(resp.status_code)
-        if resp.status_code == 429:
-            break
-    assert 429 in statuses, f"429 が観測されなかった: {statuses}"
-    limiter.reset()
+    limiter.reset()
+    try:
+        statuses: list[int] = []
+        for _ in range(8):
+            resp = client.post(
+                "/api/github-link/run",
+                json={"include_forks": False},
+                headers=headers,
+            )
+            statuses.append(resp.status_code)
+            if resp.status_code == 429:
+                break
+        assert 429 in statuses, f"429 が観測されなかった: {statuses}"
+    finally:
+        limiter.reset()
@@
-    limiter.reset()
-    statuses: list[int] = []
-    for _ in range(13):
-        resp = client.post("/api/blog/accounts/nonexistent/sync", headers=headers)
-        statuses.append(resp.status_code)
-        if resp.status_code == 429:
-            break
-    assert 429 in statuses, f"429 が観測されなかった: {statuses}"
-    limiter.reset()
+    limiter.reset()
+    try:
+        statuses: list[int] = []
+        for _ in range(13):
+            resp = client.post("/api/blog/accounts/nonexistent/sync", headers=headers)
+            statuses.append(resp.status_code)
+            if resp.status_code == 429:
+                break
+        assert 429 in statuses, f"429 が観測されなかった: {statuses}"
+    finally:
+        limiter.reset()

Also applies to: 45-53

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/tests/security/test_rate_limit.py` around lines 23 - 35, The test
mutates global rate-limit state by calling limiter.reset() only on the success
path; wrap the request loop and assertions in a try/finally and move the shared
limiter.reset() into the finally block so the limiter is always reset even if a
request or assertion fails; apply the same change to the second similar block
(the one around lines 45-53) that uses client.post, statuses, and limiter.reset
so both test sections always call limiter.reset() regardless of exceptions.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes